[lldb-dap] Add invalidated event #157530

DrSergei · 2025-09-08T18:37:09Z

This patch fixes the problem, when after a setVariable request pointers and references to the variable are not updated. VSCode doesn't send a variables request after a setVariable request, so we should trigger it explicitly viainvalidated event .Also, updated writeMemory request in similar way.

llvmbot · 2025-09-08T18:37:51Z

@llvm/pr-subscribers-lldb

Author: Druzhkov Sergei (DrSergei)

Changes

This patch fixes the problem, when after a setVariable request pointers and references to the variable are not updated. VSCode doesn't send a variables request after a setVariable request, so we should trigger it explicitly viainvalidated event .Also, updated writeMemory request in similar way.

Full diff: https://github.com/llvm/llvm-project/pull/157530.diff

8 Files Affected:

(modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3)
(modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+18-2)
(modified) lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py (+1-3)
(modified) lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py (+10-24)
(modified) lldb/tools/lldb-dap/EventHelper.cpp (+22)
(modified) lldb/tools/lldb-dap/EventHelper.h (+10)
(modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+4)
(modified) lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp (+6)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 66aa070a537e0..db3efd3bcb30c 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -215,6 +215,7 @@ def __init__(
         self.terminated: bool = False
         self.events: List[Event] = []
         self.progress_events: List[Event] = []
+        self.invalidated_event: Event = None
         self.reverse_requests: List[Request] = []
         self.module_events: List[Dict] = []
         self.sequence: int = 1
@@ -440,6 +441,8 @@ def _handle_event(self, packet: Event) -> None:
         elif event == "capabilities" and body:
             # Update the capabilities with new ones from the event.
             self.capabilities.update(body["capabilities"])
+        elif event == "invalidated":
+            self.invalidated_event = packet
 
     def _handle_reverse_request(self, request: Request) -> None:
         if request in self.reverse_requests:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index fffd4c23d6fcd..a0a009ae6cc9a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -241,6 +241,13 @@ def verify_commands(self, flavor: str, output: str, commands: list[str]):
                 f"Command '{flavor}' - '{cmd}' not found in output: {output}",
             )
 
+    def verify_invalidated_event(self, expected_areas):
+        event = self.dap_server.invalidated_event
+        self.dap_server.invalidated_event = None
+        self.assertIsNotNone(event)
+        areas = event["body"].get("areas", [])
+        self.assertEqual(set(expected_areas), set(areas))
+
     def get_dict_value(self, d: dict, key_path: list[str]) -> Any:
         """Verify each key in the key_path array is in contained in each
         dictionary within "d". Assert if any key isn't in the
@@ -352,13 +359,20 @@ def get_local_as_int(self, name, threadId=None):
         else:
             return int(value)
 
+    def set_variable(self, varRef, name, value, id=None):
+        """Set a variable."""
+        response = self.dap_server.request_setVariable(varRef, name, str(value), id=id)
+        if response["success"]:
+            self.verify_invalidated_event(["variables"])
+        return response
+
     def set_local(self, name, value, id=None):
         """Set a top level local variable only."""
-        return self.dap_server.request_setVariable(1, name, str(value), id=id)
+        return self.set_variable(1, name, str(value), id=id)
 
     def set_global(self, name, value, id=None):
         """Set a top level global variable only."""
-        return self.dap_server.request_setVariable(2, name, str(value), id=id)
+        return self.set_variable(2, name, str(value), id=id)
 
     def stepIn(
         self,
@@ -577,4 +591,6 @@ def writeMemory(self, memoryReference, data=None, offset=0, allowPartial=False):
         response = self.dap_server.request_writeMemory(
             memoryReference, encodedData, offset=offset, allowPartial=allowPartial
         )
+        if response["success"]:
+            self.verify_invalidated_event(["all"])
         return response
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
index f51056d7020c6..7c9ad0c0f75ee 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -72,9 +72,7 @@ def test_memory_refs_set_variable(self):
         ptr_value = self.get_local_as_int("rawptr")
         self.assertIn(
             "memoryReference",
-            self.dap_server.request_setVariable(1, "rawptr", ptr_value + 2)[
-                "body"
-            ].keys(),
+            self.set_local("rawptr", ptr_value + 2)["body"].keys(),
         )
 
     @skipIfWindows
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
index a3a4bdaaf40a6..13a694602f230 100644
--- a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
+++ b/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
@@ -298,7 +298,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         # Set a variable value whose name is synthetic, like a variable index
         # and verify the value by reading it
         variable_value = 100
-        response = self.dap_server.request_setVariable(varRef, "[0]", variable_value)
+        response = self.set_variable(varRef, "[0]", variable_value)
         # Verify dap sent the correct response
         verify_response = {
             "type": "int",
@@ -315,7 +315,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         # Set a variable value whose name is a real child value, like "pt.x"
         # and verify the value by reading it
         varRef = varref_dict["pt"]
-        self.dap_server.request_setVariable(varRef, "x", 111)
+        self.set_variable(varRef, "x", 111)
         response = self.dap_server.request_variables(varRef, start=0, count=1)
         value = response["body"]["variables"][0]["value"]
         self.assertEqual(
@@ -341,27 +341,15 @@ def do_test_scopes_variables_setVariable_evaluate(
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
         # Now we verify that we correctly change the name of a variable with and without differentiator suffix
-        self.assertFalse(self.dap_server.request_setVariable(1, "x2", 9)["success"])
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:0", 9)["success"]
-        )
+        self.assertFalse(self.set_local("x2", 9)["success"])
+        self.assertFalse(self.set_local("x @ main.cpp:0", 9)["success"])
 
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:19", 19)["success"]
-        )
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:21", 21)["success"]
-        )
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:23", 23)["success"]
-        )
+        self.assertTrue(self.set_local("x @ main.cpp:19", 19)["success"])
+        self.assertTrue(self.set_local("x @ main.cpp:21", 21)["success"])
+        self.assertTrue(self.set_local("x @ main.cpp:23", 23)["success"])
 
         # The following should have no effect
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:23", "invalid")[
-                "success"
-            ]
-        )
+        self.assertFalse(self.set_local("x @ main.cpp:23", "invalid")["success"])
 
         verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19"
         verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21"
@@ -370,7 +358,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
         # The plain x variable shold refer to the innermost x
-        self.assertTrue(self.dap_server.request_setVariable(1, "x", 22)["success"])
+        self.assertTrue(self.set_local("x", 22)["success"])
         verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22"
 
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
@@ -708,9 +696,7 @@ def test_return_variables(self):
                 self.verify_variables(verify_locals, local_variables, varref_dict)
                 break
 
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "(Return Value)", 20)["success"]
-        )
+        self.assertFalse(self.set_local("(Return Value)", 20)["success"])
 
     @skipIfWindows
     def test_indexedVariables(self):
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index ecd630cb530d6..68617dbc8a364 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -273,4 +273,26 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
   dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+json::Value toJSON(const InvalidatedArea &area) {
+  switch (area) {
+  case InvalidatedArea::eInvalidatedAreaAll:
+    return "all";
+  case InvalidatedArea::eInvalidatedAreaStacks:
+    return "stacks";
+  case InvalidatedArea::eInvalidatedAreaThreads:
+    return "threads";
+  case InvalidatedArea::eInvalidatedAreaVariables:
+    return "variables";
+  }
+  llvm_unreachable("unhandled invalidated event area!.");
+}
+
+void SendInvalidatedEvent(DAP &dap, const std::vector<InvalidatedArea> &areas) {
+  llvm::json::Object event(CreateEventObject("invalidated"));
+  llvm::json::Object body;
+  body.try_emplace("areas", areas);
+  event.try_emplace("body", std::move(body));
+  dap.SendJSON(llvm::json::Value(std::move(event)));
+}
+
 } // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 592c1b81c46af..6ac0e1bb769bc 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -11,6 +11,7 @@
 
 #include "DAPForward.h"
 #include "llvm/Support/Error.h"
+#include <vector>
 
 namespace lldb_dap {
 struct DAP;
@@ -32,6 +33,15 @@ void SendContinuedEvent(DAP &dap);
 
 void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
 
+enum InvalidatedArea : unsigned {
+  eInvalidatedAreaAll,
+  eInvalidatedAreaStacks,
+  eInvalidatedAreaThreads,
+  eInvalidatedAreaVariables
+};
+
+void SendInvalidatedEvent(DAP &dap, const std::vector<InvalidatedArea> &areas);
+
 } // namespace lldb_dap
 
 #endif
diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
index d07c0d6c9afa8..ab3129d1abd7b 100644
--- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
@@ -77,6 +77,10 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
   if (ValuePointsToCode(variable))
     body.valueLocationReference = new_var_ref;
 
+  // Also send invalidated event to signal client
+  // that some variables (e.g. references) can be changed
+  SendInvalidatedEvent(dap, {InvalidatedArea::eInvalidatedAreaVariables});
+
   return body;
 }
 
diff --git a/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
index 313f59dceab24..fa62afb3957e5 100644
--- a/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "EventHelper.h"
 #include "JSONUtils.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
@@ -93,6 +94,11 @@ WriteMemoryRequestHandler::Run(
   }
   protocol::WriteMemoryResponseBody response;
   response.bytesWritten = bytes_written;
+
+  // Also send invalidated event to signal client
+  // that some things can be changed (e.g. variables)
+  SendInvalidatedEvent(dap, {InvalidatedArea::eInvalidatedAreaAll});
+
   return response;
 }

JDevlieghere · 2025-09-08T21:36:57Z

lldb/tools/lldb-dap/EventHelper.cpp

Please create a struct to represent this event in lldb/tools/lldb-dap/Protocol/ProtocolEvents.h and add a unit test for JSON (de)serialization.

JDevlieghere · 2025-09-08T21:37:17Z

lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp

Suggested change

// Also send invalidated event to signal client

// that some things can be changed (e.g. variables)

// Also send invalidated event to signal client

// that some things can be changed. (e.g. variables)

JDevlieghere · 2025-09-08T21:37:47Z

lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp

This seems to be wrapping before the 80 column limit? Also missing period.

Suggested change

// Also send invalidated event to signal client

// that some variables (e.g. references) can be changed

// Also send invalidated event to signal client

// that some variables (e.g. references) can be changed.

walter-erquinigo

pretty nice. I just left a minor comment

walter-erquinigo · 2025-09-09T14:11:21Z

lldb/tools/lldb-dap/EventHelper.h


 void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);

+void SendInvalidatedEvent(


Add documention and mention that areas is moved within the function.
I also think it might be fine not to use &&, get a llvm::ArrayRef as argument and create a copy of it inside the implementation. We don't need to over optimize this part and instead it might be better to reduce the number of side effects (i.e. moving areas).

Thanks, fixed

da-viper

LGTM

da-viper · 2025-09-09T16:12:25Z

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

        self.terminated: bool = False
        self.events: List[Event] = []
        self.progress_events: List[Event] = []
+        self.invalidated_event: Event = None


Suggested change

self.invalidated_event: Event = None

self.invalidated_event: Optional[Event] = None

JDevlieghere

LGTM

JDevlieghere · 2025-09-09T16:54:52Z

lldb/unittests/DAP/ProtocolTypesTest.cpp

+    "threads"
+  ]
+})";
+  // Validate toJSON


Suggested change

// Validate toJSON

ashgti · 2025-09-09T16:51:09Z

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

            # Update the capabilities with new ones from the event.
            self.capabilities.update(body["capabilities"])
+        elif event == "invalidated":
+            self.invalidated_event = packet


Should this be a list in case we get more invalidate events? Or when does this reset?

We check invalidated event in verify_invalidated_event function and reset this field. This function is called inside wrappers around setVariable and writeMemory requests.

ashgti · 2025-09-09T16:55:06Z

lldb/tools/lldb-dap/Protocol/ProtocolEvents.h

+struct InvalidatedEventBody {
+  enum Area : unsigned { eAreaAll, eAreaStacks, eAreaThreads, eAreaVariables };
+
+  std::vector<Area> areas;


Should we have the fields for threadId and stackFrameId in case we add that in the future?

ashgti · 2025-09-09T16:56:53Z

lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp

+
+  // Also send invalidated event to signal client that some things
+  // (e.g. variables) can be changed.
+  SendInvalidatedEvent(dap, {InvalidatedEventBody::eAreaAll});


Does this change everything? Or just variables? I am not sure if writing into memory like this prevents changing the current stack or PC counter or not.

I think we can change return address on stack using writeMemory request

This patch fixes the problem, when after a `setVariable` request pointers and references to the variable are not updated. VSCode doesn't send a `variables` request after a `setVariable` request, so we should trigger it explicitly via`invalidated` event .Also, updated `writeMemory` request in similar way. (cherry picked from commit 770cd43)

DrSergei requested a review from JDevlieghere as a code owner September 8, 2025 18:37

llvmbot added lldb lldb-dap labels Sep 8, 2025

JDevlieghere requested review from ashgti and da-viper September 8, 2025 21:36

JDevlieghere reviewed Sep 8, 2025

View reviewed changes

DrSergei force-pushed the add-invalidated-event branch from 35117bc to 3ce62df Compare September 9, 2025 05:48

[lldb-dap] Add invalidated event

a71a195

DrSergei force-pushed the add-invalidated-event branch from 3ce62df to a71a195 Compare September 9, 2025 10:00

walter-erquinigo reviewed Sep 9, 2025

View reviewed changes

Add ArrayRef

686d127

da-viper approved these changes Sep 9, 2025

View reviewed changes

JDevlieghere approved these changes Sep 9, 2025

View reviewed changes

lldb/unittests/DAP/ProtocolTypesTest.cpp Outdated

"threads"

]

})";

// Validate toJSON

Copy link

Member

JDevlieghere Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

// Validate toJSON

ashgti reviewed Sep 9, 2025

View reviewed changes

Fix review comments

8ead4d0

ashgti approved these changes Sep 9, 2025

View reviewed changes

JDevlieghere merged commit 770cd43 into llvm:main Sep 11, 2025
9 checks passed


		void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);

		void SendInvalidatedEvent(

	self.invalidated_event: Event = None
	self.invalidated_event: Optional[Event] = None

[lldb-dap] Add invalidated event #157530

[lldb-dap] Add invalidated event #157530

Uh oh!

Conversation

DrSergei commented Sep 8, 2025

Uh oh!

llvmbot commented Sep 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

walter-erquinigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

da-viper left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants